-
Notifications
You must be signed in to change notification settings - Fork 773
feature: Hijack HTTPS by generating leaf TLS certs on the fly issued by user provided CA. #1181
Conversation
We found this is your first time to contribute to Dragonfly, @YanzheL |
b9475ff
to
e40b16a
Compare
Codecov Report
@@ Coverage Diff @@
## master #1181 +/- ##
==========================================
- Coverage 48.27% 47.89% -0.38%
==========================================
Files 116 117 +1
Lines 7263 7320 +57
==========================================
Hits 3506 3506
- Misses 3477 3534 +57
Partials 280 280
Continue to review full report at Codecov.
|
@YanzheL Thanks a lot for your PR ! This has been a known issue we haven't yet fix. The current implementation indeed requires clients to explicitly ignore certificate errors. Adding a CA to the system cert store is the right way to do it. I'll review the code later today, meanwhile would you please:
|
e40b16a
to
d3e6b2a
Compare
Thanks for your reply. I wrote a simple guide to test this feature. How to use dfclient with custom CAGenerate Self-Signed CAI recommend using CFSSL to do this step. For user's convenience, we can implement CA auto-generation in future. If you do this with OpenSSL, please make sure the x509 Key Usage Extension contains
Now we have a self-signed CA key-pair Configure dfclient to use this CAproxies:
- regx: blobs/sha256.*
hijack_https:
cert: /path/to/your/ca.crt
key: /path/to/your/ca.key
hosts: # We test some non-dockerhub HTTPS registries here.
- regx: 'quay.io'.
- regx: 'registry.gitlab.com'
- regx: 'gcr.io' Configure
|
In fact, this error is not related to this PR. Please ignore it. |
fdfd03a
to
f5ee1fb
Compare
5d34003
to
840e446
Compare
LGTM Since this change is backward compatible, I think we can merge this PR now. @YanzheL would you please rebase the 3 commits into one and add a proper commit message. Documentation and automatic CA generation are still needed though, I'll open an issue to track them. Thanks for the great work! |
840e446
to
7d351b4
Compare
…rovided CA Signed-off-by: YanzheL <lee.yanzhe@yanzhe.org>
7d351b4
to
96c3c82
Compare
Thank you for reviewing. These commits are squashed now. |
Signed-off-by: Gaius <gaius.qi@gmail.com>
Signed-off-by: YanzheL lee.yanzhe@yanzhe.org
Ⅰ. Why do you propose this PR
The current implementation of HTTPS hijacking is simple but not correct.
It uses user provided TLS server cert
df.crt
anddf.key
to decrypt HTTPS connection. However, this cert cannot be a CA, which means every TLS connection is encrypted by the same cert from user point of view.This is not a standard behavior of a HTTPS Man-In-The-Middle proxy, and will cause various issues.
The cert used by HTTPS hijacking cannot be automatically verified by applications because the Common Name (or Server-Alternative-Names) is always same and it doesn't match the host of every connection. So user have to configure their applications manually to force-ignore the TLS verification.
This prevents
dfclient
be used as a general system-level HTTPS proxy without affecting user applications.Some applications cannot be configured to trust a specific TLS cert or ignore TLS verification error (e.g. Google Chrome). Instead, the only way to achieve it is to configure them to trust the CA, or add the CA to system trust store.
II. Describe what this PR did
If
df.crt
anddf.key
is a CA key-pair, thendfclient
use it to issue leaf TLS certs for every connection whose host matches pre-configured hijacking rules.User can either add the CA to system trust store, or configure individual application to trust it.
Since the common name of leaf cert is set as the target host, the connection will be verified by user application automatically as normal.
If
df.crt
anddf.key
is not a CA key-pair, the behavior ofdfclient
is same as the old way: this cert is used in hijacking instead of generating new certs per connection.So this PR is fully backward-compatible and will NOT break user applications.
ⅡI. Does this pull request fix one issue?
Potential issues are stated above.
IV. Potential use cases
Maybe now we can cache HTTPS docker registries (Probably fixes Why does Dragonfly can not support HTTPS mirror repositories well? #525).
The dfdaemon acts as a decrypting MITM HTTPS proxy, so it can 'see' the request body of docker image pull requests to remote private HTTPS registries. If we can see the body, then we can cache it as well.
Generic HTTPS caching proxy, just like squid. Cache anything in a distributed way, and not just for HTTP contents. We can also use dfdaemon to speed up normal webpage loading in browser.
V. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)
I'm working on unit tests, but currently I don't have much time....
For now, I just tested this feature in container, and everything seems good.
VI. Describe how to verify it
Prepare a self-signed CA with private key.
Configure dfdaemon to use this key pair, and also configure the hijack rules, proxy rules.
Setup dfdaemon as your system's http(s) proxy.
Check the TLS cert return by target site.
This command will report that the TLS cert of
alibaba.com
is signed by your CA.The dfdaemon log will indicate that it is downloading png files of
alibaba.com
ⅤII. Special notes for reviews
This PR reuses CA's private key (and signature algorithm) to generate per-connection certs. I think it can save key-generation overhead and there is no need to use new TLS private key for every connection since the generated cert is temporal (Default valid time is 24 hours).
As for security, if the private key of CA is leaked someway, generating new private key for every connection will not improve security (maybe???). So reusing CA private key does not bring much security issues.
If this is not appropriate, I can change it.